[212_2703] Fix gs path quoting for paths with spaces on macOS#2859
[212_2703] Fix gs path quoting for paths with spaces on macOS#2859dorodb-web22 wants to merge 1 commit intoMoganLab:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes Ghostscript command-line construction so PS/EPS rendering works on macOS when temp paths contain spaces (e.g., user home paths like .../John Doe/...).
Changes:
- Replace
sys_concretize(url)withraw_quote(concretize(url))for Ghostscript file paths ings_utilities.cpp. - Update the Ghostscript PDF size probing path to quote
$TEXMACS_PATH/misc/convert/pdf_info.ps. - Add a developer note (
devel/212_2703.md) describing the issue, rationale, and test steps.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
src/Plugins/Ghostscript/gs_utilities.cpp |
Updates how file paths are embedded in Ghostscript command strings to handle spaces. |
devel/212_2703.md |
Documents reproduction, fix rationale, and manual test procedure for #2703. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // real eps pages with proper bounding boxes have been recognized | ||
| // before this and will have their BoundingBox respected | ||
| cmd << sys_concretize (image); | ||
| cmd << raw_quote (concretize (image)); |
There was a problem hiding this comment.
raw_quote(concretize(image)) is not shell-safe on macOS/Linux: raw_quote just wraps in double quotes and does not escape $, backticks, or ". Since lolly::system::call uses wordexp(..., flags=0) (command substitution enabled), a crafted filename could trigger expansion/command execution. For positional file args, sys_concretize(image) already handles spaces via backslash-escaping while also escaping metacharacters; consider keeping sys_concretize here and only changing the truly GS-option-value cases, or introduce a dedicated safe quoting/escaping helper for wordexp.
| cmd << raw_quote (concretize (image)); | |
| cmd << sys_concretize (image); |
| if (gs_version () >= 9.50) | ||
| cmd << "--permit-file-read=" << sys_concretize (image) << " "; | ||
| cmd << "--permit-file-read=" << raw_quote (concretize (image)) << " "; | ||
| cmd << "-dNODISPLAY -q -sFile="; | ||
| cmd << sys_concretize (image); | ||
| cmd << raw_quote (concretize (image)); | ||
| cmd << " pdf_info.ps"; |
There was a problem hiding this comment.
Using raw_quote(concretize(...)) inside --permit-file-read= / -sFile= is unsafe with the current subprocess implementation: on macOS/Linux lolly::system::call relies on wordexp without WRDE_NOCMD, and raw_quote does not escape $/backticks/", so paths like $(...) can be expanded. Please switch to a shell-safe quoting function (e.g., single-quote with proper escaping, or escape $, `, \, and " inside double quotes) or avoid wordexp for argument splitting.
| cmd << "-dQUIET -dNOPAUSE -dBATCH -dSAFER "; | ||
| cmd << "-sDEVICE=" << eps_device (); | ||
| cmd << " -sOutputFile=" << sys_concretize (eps) << " "; | ||
| cmd << " -sOutputFile=" << raw_quote (concretize (eps)) << " "; | ||
| if (suffix (image) == "pdf") { |
There was a problem hiding this comment.
raw_quote(concretize(eps)) does not escape $/backticks/" and is therefore not safe to embed in a command string that is later parsed by wordexp (used by lolly::system::call on macOS/Linux). This is a potential command-injection vector via crafted filenames/paths. Please use a shell-safe quoting/escaping routine for paths here (or avoid string-based command parsing altogether).
| cmd << " -sOutputFile=" << raw_quote (concretize (ps)) << " "; | ||
| cmd << raw_quote (concretize (doc)); | ||
| cmd << " -c \"[ /Title (" << as_string (tail (ps)) << ") /DOCINFO pdfmark\" "; |
There was a problem hiding this comment.
The new raw_quote(concretize(ps/doc)) usage is not safe with the current subprocess implementation on macOS/Linux: raw_quote does not escape $, backticks, or embedded quotes, and lolly::system::call uses wordexp with command substitution enabled. A malicious filename could be expanded/executed. Please switch to a shell-safe quoting approach (or pass argv without wordexp).
| string cmd= gs_prefix (); | ||
| cmd << "-q -sDEVICE=x11alpha -dBATCH -dNOPAUSE -dSAFER -dNOEPS "; | ||
| cmd << sys_concretize (image); | ||
| cmd << raw_quote (concretize (image)); | ||
| lolly::system::call (cmd); |
There was a problem hiding this comment.
raw_quote(concretize(image)) is not a safe way to quote a path for execution on macOS/Linux because raw_quote does not escape $/backticks/", and lolly::system::call uses wordexp (command substitution enabled). This allows command substitution if an image path contains $(...)/backticks. Please use a shell-safe quoting/escaping function for paths passed to subprocesses.
|
@MoonL79 I'm inclined to keep the current approach because:
That said, if u would prefer a more conservative approach, I can limit raw_quote to |
|
Thank you for your work! I will get back to you today. |
|
Thanks for the clarification. I agree the space-in-path bug is real and your change fixes that behavior for GS option values. My concern is the security regression: raw_quote(concretize(...)) only adds double quotes, and on macOS/Linux our command path still goes through wordexp(..., 0), so Could you please update this to a shell-safe + GS-safe approach instead of plain raw_quote? At minimum, please add/cover cases for paths containing: space, $, backticks, ", ', and , for both standalone args and -sOutputFile= / -sFile= / --permit-file-read=forms. |
Problem
On macOS, MoganSTEM fails to render PS/EPS images when the temp path contains
spaces (e.g., when the user's full name has a space, like
/var/folders/.../John Doe/...).Ghostscript receives malformed arguments and cannot process the file.
Closes #2703
Root Cause
sys_concretize(url) calls escape_sh(concretize(url)), which on macOS/Linux
backslash-escapes spaces:
gs -sOutputFile=/tmp/path\ with\ spaces/file.eps
The shell correctly passes standalone args with backslash-escapes, but Ghostscript
parses its own option values (
-sOutputFile=...,-sFile=...,--permit-file-read=...)as raw strings — it does not re-interpret shell backslash escapes. So GS splits
at the space and receives a malformed path.
Fix
Replace sys_concretize(url) with raw_quote(concretize(url)) at all path-in-option
sites in gs_utilities.cpp. Double-quoting passes the full path as one token:
gs -sOutputFile="/tmp/path with spaces/file.eps"
This is consistent with gs_prefix(), which already uses raw_quote(gs_executable())
on Windows for the same reason.
Changed functions: gs_image_size, gs_PDFimage_size, gs_to_eps, gs_to_ps, tm_gs.
How to Test
xmake build stem && xmake run stem